chore: converts remaining file api methods to async iterators#2517
chore: converts remaining file api methods to async iterators#2517
Conversation
achingbrain
force-pushed
the
add-remaining-file-async-iterator-methods
branch
from
67b9a27 to
d9986ee
Compare
| // can't use callbackify because if `data` is a pull stream | ||
| // it thinks we are passing a callback. This is why we can't have nice things. | ||
| return (data, options, callback) => { | ||
| return function add (data, options, callback) { |
There was a problem hiding this comment.
Why move from arrow func to named func here?
There was a problem hiding this comment.
To have function names in any stack traces emitted making them a teensy bit easier to read.
| const invalidPath = null | ||
| const stream = ipfs.getReadableStream(invalidPath) | ||
|
|
||
| stream.on('data', () => {}) |
There was a problem hiding this comment.
Interesting. Something changed for now it requiring to start pumping for the error to be emitted?
There was a problem hiding this comment.
In the old implementation, converting the pull stream to a readable stream calls s.sink(read) in pull-stream-to-stream which calls drain() on the next tick which causes data to start flowing, potentially before the consuming code has set up pipes or data event listeners.
I've opened pull-stream/pull-stream-to-stream#7 as this looks like a bug, at least, it's not consistent with the node docs.
The new implementation doesn't do this conversion so doesn't have the same bug.
This switches to async iterator version of ipfs.add (introduced to js-ipfs in ipfs/js-ipfs#2517) and ensures Node streams are replaced by deterministic version of readable-stream Closes #757
* fix: /api/v0/add in Brave This switches to async iterator version of ipfs.add (introduced to js-ipfs in ipfs/js-ipfs#2517) and ensures Node streams are replaced by deterministic version of readable-stream Closes #757 * fix(cid): fast finish + allow osx to fail
alanshaw
left a comment
alanshaw
left a comment
There was a problem hiding this comment.
🥳 woohoo! Just a few feedback points but this look great!
| }) | ||
| }) | ||
| }) | ||
| .finally(() => { |
There was a problem hiding this comment.
Wow, I did not realise promises had a finally...
There was a problem hiding this comment.
Putting the Java into Javascript 😆
|
ping @achingbrain 🙏 |
Co-Authored-By: Alan Shaw <alan.shaw@protocol.ai>
3 participants
Depends on ipfs-inactive/js-ipfs-http-client#1124